Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update keymap on keymap change #1164

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

user202729
Copy link
Member

@user202729 user202729 commented Oct 27, 2020

Summary of changes

Listens for XMappingEvents and call _update_keymap whenever the keymap changes.

Closes #967. Might also fix #650.

Relies on the other pull request for self._display_lock.

Problem: for some reason, it refreshes the key map every time I use a different keyboard (in case I have more than one keyboard plugged in the machine -- which is usually the case with the built-in keyboard and a steno keyboard)

Pull Request Checklist

  • Changes have tests
  • News fragment added in news.d. See documentation for details

@benoit-pierre
Copy link
Member

Sorry for the delay, will review later (maybe after 4.0.0 is released): when I investigated updating the layout on keymap change, I notice some issues due to the fact that the code does not handle XKB, which meant in my case I would get a lot of keymap changes because of the switch between the events sent by my physical keyboard, and the XTest keyboard (which use different layouts). I realize that my use case might be special (multiple physical keyboards with different layouts), so maybe that's not a valid reason for not updating on change. The alternative might be proper XKB support, but python-xlib does not support that (and adding support would be a lot of work).

@user202729
Copy link
Member Author

About xkb... I think that it is also the cause of #1189 so to fix that without xkb support this one is necessary.

Xref: Switching keyboards causes issues in other programs not supporting xkb too (#1030 )

@user202729
Copy link
Member Author

user202729 commented Apr 5, 2021

Conflict, okay...

In retrospect, it isn't a very good idea to use inheritance all the time. In fact it's forbidden to override/define anything other than run and __init__ in a subclass of Thread, and in the future when/if output plugin is implemented, it's going to be a mess (with multiple inheritance, KeyboardEmulationBase and XEventLoop).

Edit: okay, thread inheritance is not my fault.

The @with_display_lock decorators are still very hard to place correctly.

@user202729
Copy link
Member Author

user202729 commented Jun 27, 2023

Rebased to latest master.

Remarks:

  • Now (after the rebase) the KeyboardCapture object contains 2 display objects, one inside the event loop and one for (sending events|update keymap). The latter one needs a separate lock (just in case), which results in the code being duplicated from XEventLoop to KeyboardEmulation.

    (it may be a better idea to make 3 display objects instead so one lock is avoided... but this crucially relies on XEventLoop's implementation to all on_event functions sequentially...)

  • The event loop is started, but never canceled. (as before the rebase)

  • Now I see Benoit's pointed out problem that the keymap is refreshed on every stroke. Not good.

    Note that this only happen with the normal keyboard machine, not if the machine use a serial protocol. Personally, I use a hack that makes the keyboard float so this is not a problem either.

  • Crucially, it may happen that the keyboard mapping is different for different keyboard, and Plover sets the mapping for the real keyboard but send events using the xtest keyboard which leads to the problem being "unfixably broken" even after a Plover restart.

    In this case it would be necessary to exit Plover, setxkbmap us, reopen Plover.


in any case, some investigation is needed. pull request is not ready.


Actually, I think a possibility is to suppress the whole keyboard instead of just the subset of keys that are used for steno using xinput float; that way when a keyboard simulation is done then it does not switch back-and-forth the layout between the normal keyboard and the xtest keyboard. I use a little hack on my side to do that, and it works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XKB groups and layout changes are not supported on Linux ( ͡° ͜ʖ ͡°) sometimes written as ( ° °)
2 participants